Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: bring elaborator in line with kernel for primitive projections #5822

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Oct 23, 2024

The kernel supports primitive projections for all inductive types with one construtor. The elaborator was assuming primitive projections only work for "structure-likes", non-recursive inductive types with no indices.

Enables numeric projection notation for general one-constructor inductives.

Extracted from #5783.

@kmill
Copy link
Collaborator Author

kmill commented Oct 23, 2024

@leodemoura These changes are to support the inductive types created by the structure command in #5783. I have carefully reviewed all uses of the various structure-related functions to be sure that the elaborator is not assuming isStructure means isStructureLike, and I have allowed primitive projections in the elaborator to use one-constructor types, just like in the kernel.

Without these changes, recursive structures lead to some elaborator panics, and they do not support numerical projection notation.

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Oct 23, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Oct 23, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 23, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Oct 23, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Oct 23, 2024

Mathlib CI status (docs):

  • ✅ Mathlib branch lean-pr-testing-5822 has successfully built against this PR. (2024-10-23 21:14:42) View Log
  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 9847923f9be5de968f10ed7c7493e3ca0072abce --onto c50f04ace0b591027ce48db93d09dbf3e6199e9f. (2024-10-28 08:03:33)
  • ✅ Mathlib branch lean-pr-testing-5822 has successfully built against this PR. (2024-10-31 03:59:59) View Log

kmill added a commit to kmill/lean4 that referenced this pull request Oct 25, 2024
Refactors the `structure` command to support recursive structures. These are disabled for now, pending additional elaborator support in leanprover#5822. This refactor is also a step toward `structure` appearing in `mutual` blocks.

Error reporting is now more precise, and this fixes an issue where general errors could appear on the last field.

Closes leanprover#2512
github-merge-queue bot pushed a commit that referenced this pull request Oct 25, 2024
Refactors the `structure` command to support recursive structures. These
are disabled for now, pending additional elaborator support in #5822.
This refactor is also a step toward `structure` appearing in `mutual`
blocks.

Error reporting is now more precise, and this fixes an issue where
general errors could appear on the last field. Adds "don't know how to
synthesize placeholder" errors for default values.

Closes #2512
tobiasgrosser pushed a commit to opencompl/lean4 that referenced this pull request Oct 27, 2024
Refactors the `structure` command to support recursive structures. These
are disabled for now, pending additional elaborator support in leanprover#5822.
This refactor is also a step toward `structure` appearing in `mutual`
blocks.

Error reporting is now more precise, and this fixes an issue where
general errors could appear on the last field. Adds "don't know how to
synthesize placeholder" errors for default values.

Closes leanprover#2512
The kernel supports primitive projections for all inductive types with one construtor. The elaborator was assuming primitive projections only work for "structure-likes", non-recursive inductive types with no indices.

Enables numeric projection notation for general one-constructor inductives.

Extracted from leanprover#5783.
Such types can be used with primitive projections.
See also `Lean.matchConstStructLike` for a more restrictive version.
-/
@[inline] def matchConstStructure [Monad m] [MonadEnv m] [MonadError m] (e : Expr) (failK : Unit → m α) (k : InductiveVal → List Level → ConstructorVal → m α) : m α :=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rename these functions in the future? It is ok to merge with the current names, but I guess we will get confused in the future. I am referring to matchConstStructure and matchConstStructureLike

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: matchConstStructure and matchConstNonRecStructure

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue to track this suggestion #5891

@kmill kmill enabled auto-merge October 31, 2024 02:53
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Oct 31, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 31, 2024
@kmill kmill added this pull request to the merge queue Oct 31, 2024
Merged via the queue into leanprover:master with commit 03c6e99 Oct 31, 2024
14 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Oct 31, 2024
Now that the elaborator supports primitive projections for recursive
inductive types (#5822), enable defining recursive inductive types with
the `structure` command, which was set up in #5842.

Example:
```lean
structure Tree where
  n : Nat
  children : Fin n → Tree

def Tree.size : Tree → Nat
  | {n, children} => Id.run do
    let mut s := 0
    for h : i in [0 : n] do
      s := s + (children ⟨i, h.2⟩).size
    pure s
```

Note for kernel re-implementors: recursive structures are exercising the
kernel feature where primitive projections are valid for one-constructor
inductive types in general, so long as the structure isn't a `Prop` and
doesn't have any non-`Prop` fields, not just ones that are non-indexed
and non-recursive.

Closes #2512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants